TODO remove this line before merging

do you want it to run on every PR? consider running it the same times as the docker build tests (merges to master or with force:integration enabled)

I've updated to the conditions for docker.yml.. unclear to me if force:integration will pick this up but agree with the suggested strategy

do you want it to run on every PR? consider running it the same times as the docker build tests (merges to master or with force:integration enabled)

pretty sure we should never need sudo in GH actions.

Had source from here but since removed

why mess with a matrix?

is this a bug in starship? if so, can we get a fix upstream?

Yes, there was a breaking change in the 0.2.x helm chart release. I opened an issue: cosmology-tech/StarshipJS#3

You really want delete to fail silently?

It was helpful when I was working out the correct pattern here. I've since refactored in support.ts to:

const makeKeyring: (e2eTools: Pick<E2ETools, "addKey" | "deleteKey">) => Promise<{
    setupTestKeys: (keys?: string[]) => Promise<Record<string, string>>;
    deleteTestKeys: () => Promise<string[]>;
}>

If a test needs keys it can call setupTestKeys in test.before() and .deleteTestKeys() in test.after()

Updated to this to appease to the TS compiler:

if (!bondDenom) {
    t.fail('Bond denom not found.');
  } else {
    const { balance } = await queryClient.queryBalance(addr, bondDenom);
    t.deepEqual(balance, { denom: bondDenom, amount: '10000000000' });
  }

here's hoping we get to Ava 6 soon which terminates on failed assertions, thus narrowing the code following

not really :)

please rename to be accurate. e.g. kubectlBinary

I think we'll want to DRY this with other helpers and I'd also like to simplify it. but this works for now

I'm a little surprised we don't have exports that do this already

Following this guide everything works except the two steps below. Would be helpful to provide an example:

I considered adding a make target for adding a new key instead of the kubectl command, but not sure if it's useful beyond one-offs.

Thanks, added this note as a note

I think because these are make vars, not envvars, they did not work as is in my shell. I think the correct COIN amount is 10000000ubld. I needed to instead do:

Good catch, thank you. I've applied these changes. Also upped to 20000000ubld so there's 10 BLD left over for gas.

This did not work for me because --from key needs to match ADDR. We need to pass whatever key name matches ADDR:

thanks - fixed

We shouldn't be publishing this, so it doesn't need a version. We shouldn't be importing from it so it doesn't need a name.

It does need this to prevent publishing:

Was thinking external developers might want to depend on this at some point, but suppose it could be viewed as boilerplate instead. Can cross that bridge when we come to it.

please match the rest of the repo

Not a big deal for the other deps but it'll be surprising to a developer if some code works in one place and not the other

Agree it would be nice. 5.5.0-beta doesn't seem to play nicely here for whatever reason

consider using Ava 6 at this outset, so there's not more work to migrate later

please document with an XXX to help someone debugging the race or who wants to clean it up

would be more clear. I imagine you used the if to get type narrowing for the queryBalance call. This is a reason to adopt Ava 6. IIUC its assertions also do type assertions so lines after t.truthywill know bondDenom is not nullish.

Ava 6. IIUC its assertions also do type assertions so lines after t.truthywill know bondDenom is not nullish.

No luck here, seemingly. I used the non-null-assertion-operator to make this more readable and drop the if/else

confirmed, though I can't make sense of it. There clearly is a type assertion.

well done to include

please rename to be accurate. e.g. kubectlBinary

I wondered if we should do this throughout the repo, but I think this accomplishes the same and more: